Skip to content

Conversation

@newtork
Copy link
Contributor

@newtork newtork commented Aug 15, 2025

Context

Related

Instead of making the API more strict by adding exception types for different error cases, we can also reduce it to just one.

Feature scope:

  • Similar to OpenAI exception type(s)
  • Exception refactoring
      ClientException
        OrchestrationClientException
    -     OrchestrationFilterException
    -       Input
    -       Output
  • No breaking changes, we haven't released the experimental exception classes yet.

Definition of Done

  • Functionality scope stated & covered
  • Tests cover the scope above
  • Error handling created / updated & covered by the tests above
  • Aligned changes with the JavaScript SDK
  • Documentation updated
  • Release notes updated

@newtork newtork added the please-review Request to review a pull-request label Aug 15, 2025
@newtork newtork marked this pull request as draft August 15, 2025 10:56
Copy link
Contributor

@CharlesDuboisSAP CharlesDuboisSAP left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer this to the alternative. I also like what we have currently.

Copy link
Member

@rpanackal rpanackal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Major)

I don't see how the user can tell whether its the input or output filter that failed.

try {
   response = client.chatCompletion(prompt, configWithIOFilter)
   content = response.getContent();
} catch (OrchestrationClientException e) {
 // ...
}

I would expect something like isInputFilterException() or the like.. so the user would know which methods to call afterwards.

Currently, the user would have to try-catch the call to chatCompletion() and getContent() separately.

Copy link
Member

@rpanackal rpanackal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, I agree with Charles that this approach is preferred over the one in chore: Improve exception accessors.

@newtork
Copy link
Contributor Author

newtork commented Sep 4, 2025

i'm also okay with current state in main branch

@newtork newtork closed this Sep 4, 2025
@CharlesDuboisSAP CharlesDuboisSAP deleted the orchestration-exception-reduction branch October 22, 2025 11:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

please-review Request to review a pull-request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants